Make client requests type safe, unmarshal #2099
Merged
+237
−41
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces type-safe server-to-client requests by leveraging RequestInfo structs with generic type parameters. The key improvement is that client request responses are now properly unmarshaled using type-aware decoding instead of relying on Go's default JSON unmarshaling.
Key Changes:
- Replaced
sendRequestwith genericsendClientRequestfunction that usesRequestInfo[Req, Resp]for type safety - Changed
ResponseMessage.Resultfield fromanytojsontext.Valueto defer deserialization - Added
unmarshalResultfunction (and generator code) to properly unmarshal response results based on the request method
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/lsp/server.go | Migrated all client request calls from sendRequest to type-safe sendClientRequest, properly typed response handling |
| internal/lsp/lsproto/lsp.go | Added UnmarshalResult, NewRequestMessage, and NewNotificationMessage methods to support type-safe request handling |
| internal/lsp/lsproto/lsp_generated.go | Generated unmarshalResult function with method-specific unmarshaling logic for all LSP request types |
| internal/lsp/lsproto/jsonrpc.go | Changed ResponseMessage.Result to jsontext.Value for deferred deserialization; removed old helper functions |
| internal/lsp/lsproto/_generate/generate.mts | Added generator logic for unmarshalResult function to handle response type unmarshaling |
| internal/fourslash/fourslash.go | Updated test helpers to use new NewRequestMessage and NewNotificationMessage methods from info structs |
gabritto
approved these changes
Nov 17, 2025
andrewbranch
approved these changes
Nov 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes server-to-client requests typesafe using the same
infostructs used everywhere else.As part of this, I figured out that our client requests have not been decoding properly at all; they have just been going through the default Go json decoding. This has not bit us because we either ignore the result, or we wanted
[]any(for settings), so it just so happened to work. This PR also fixes that problem, so we are now actually decoding the responses.